source/slang/slang-ir-legalize-varying-params.cpp (1)

821-841: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Nested mixed hit-input structs still bypass the PrimitiveID rewrite.

This classification only recurses through primitiveIDOnlyStructFields. A nested field like Inner { uint pid : SV_PrimitiveID; float2 bary : SV_Barycentrics; } is treated as ordinaryFields, so Line 840 returns false before any use-walking happens and inner.pid never reaches the new PrimitiveID substitution path.

Example: struct Outer { Inner inner; }; [shader("closesthit")] void main(Outer o) { use(o.inner.pid, o.inner.bary.x); } leaves the nested SV_PrimitiveID untouched.

Suggested fix: recurse into nested mixed structs as well and rebuild the surviving ordinary subtree at each level, or reject nested mixed carriers earlier with a diagnostic until that rewrite exists.

source/slang/slang-ir-legalize-varying-params.cpp (1)

1098-1128: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preflight all PrimitiveID replacements before narrowing nested field-access types.

This helper mutates fieldExtract/fieldAddress types at Line 1098 before it proves that every
SV_PrimitiveID field can be materialized. If emitRayTracingPrimitiveIndexValue() later returns
nullptr for one field, the function returns false at Lines 1116-1117 or Lines 1127-1128 after
already changing IR types, so the caller observes “legalization failed” with partially rewritten IR
still in the entry point.

Suggested fix

-    for (auto update : fieldAccessTypeUpdates)
-    {
-        update.inst->setFullType(
-            replaceParamValueType(builder, update.inst->getFullType(), update.valueType));
-    }
-
     List<IRStructField*> replacementFields;
     List<IRInst*> replacementValues;
     auto getPrimitiveIDValueReplacement = [&](IRStructField* field) -> IRInst*
     {
         for (Index i = 0; i < replacementFields.getCount(); i++)
@@
         replacementFields.add(field);
         replacementValues.add(valueReplacement);
         return valueReplacement;
     };
+
+    for (auto access : primitiveIDFieldAccesses)
+    {
+        if (!getPrimitiveIDValueReplacement(access.field))
+            return false;
+    }
+
+    for (auto update : fieldAccessTypeUpdates)
+    {
+        update.inst->setFullType(
+            replaceParamValueType(builder, update.inst->getFullType(), update.valueType));
+    }
 
     for (auto access : primitiveIDFieldAccesses)
     {
         auto valueReplacement = getPrimitiveIDValueReplacement(access.field);
         if (!valueReplacement)

source/slang/slang-ir-legalize-varying-params.h (1)

130-139: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🟡 Gap: valueEmitter parameter undocumented in function comment

The doc comment at lines 130-133 describes the struct-parameter rewriting behavior but does not explain the new optional valueEmitter parameter added at line 139. A caller cannot determine from the header:

• What happens when valueEmitter is nullptr (default backend-agnostic emission?)
• When a caller should provide a custom emitter (backend-specific primitive-index intrinsic?)
• What the callback receives and must return

Suggestion: Extend the doc comment to document the valueEmitter parameter, e.g., add before line 134:

/// When `valueEmitter` is non-null, its callback is invoked for each primitive-ID
/// field to emit the replacement value; when null, default emission is used.

Source: Coding guidelines

source/slang/slang-ir-legalize-varying-params.cpp (1)

683-719: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Array-carried SV_PrimitiveID still bypasses this legalization.

This recursion only descends into direct IRStructType fields. A hit-stage input like struct Inner { uint pid : SV_PrimitiveID; }; struct Outer { Inner items[2]; }; falls through the ordinary-field branch here, so info->containsPrimitiveID stays false and tryLegalizeRayTracingPrimitiveIDStructParam() leaves the parameter untouched. That skips the PrimitiveID rewrite for items[i].pid and can leak an unsupported signature to later backend passes.

Suggested fix:

// Pseudocode
if (auto nestedStructType = as<IRStructType>(field->getFieldType()))
{
    ...
}
else if (auto arrayType = as<IRArrayTypeBase>(field->getFieldType()))
{
    // Recurse into the element type/layout when the element is a struct,
    // or diagnose explicitly if aggregate carriers are intentionally unsupported.
}

Source: Coding guidelines